Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resolve issue with batching MRs and forked MRs (#302) #322

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sg70
Copy link
Contributor

@sg70 sg70 commented Oct 1, 2021

Since all forked MRs are prepared to local branches, the MR
acceptance procedure needs to be also local.

  • fix batching MRs when source branch has different repo url
  • add debug messages
  • update unit tests
  • add unit test for use case forked MRs
  • remove credentials from debug log

Changes:

/tmpiq7raaob/tmpb84bjxf0 # git merge origin/dummy21 --ff --ff-only
merge: origin/dummy21 - not something we can merge

Into:

/tmpiq7raaob/tmpb84bjxf0 # git merge dummy21 --ff --ff-only
Updating b51f541..9525b89
Fast-forward
 21 | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 21

Signed-off-by: Sascha Binckly [email protected]

@sg70
Copy link
Contributor Author

sg70 commented Oct 1, 2021

@qqshfox we have used this custom version for a couple of month without any issue. It would be nice to have this merged into the project.

@bigc2000 we created the issue and worked on a solution as well. We have our fix running internally since May. Let's team up on one PR.

# delay in case Gitlab is slow to respond
waiting_time_in_secs = 20
log.debug('Waiting for %s secs for MR !%s to get status update', waiting_time_in_secs, batch_mr.iid)
time.sleep(waiting_time_in_secs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for 20 seconds to get status update seems brutal. Would it be better to have a function to poll the status and have an interval and a limit? It can be used for other sleeps as well.

Since all forked MRs are prepared to local branches, the MR
acceptance procedure needs to be also local.

* fix batching MRs when source branch has different repo url
* fix unit tests
* add unit test for use case forked MRs
* remove credentials from debug log

Changes:
/tmpiq7raaob/tmpb84bjxf0 # git merge origin/dummy21 --ff --ff-only
merge: origin/dummy21 - not something we can merge

Into:
/tmpiq7raaob/tmpb84bjxf0 # git merge dummy21 --ff --ff-only
Updating b51f541..9525b89
Fast-forward
 21 | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 21

Signed-off-by: Sascha Binckly <[email protected]>
@sg70 sg70 force-pushed the issue-302-fix-batching-mrs-for-forking-workflow branch from f1bb40f to 241409d Compare March 30, 2022 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants